Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix configure.ac : unused command line argument triggers erros #214

Open
wants to merge 1 commit into
base: gcos4gnucobol-3.x
Choose a base branch
from

Conversation

ddeclerck
Copy link
Contributor

@ddeclerck ddeclerck commented Jan 29, 2025

As mentionned in a previous comment, unused arguments trigger errors in the configure script :

error: argument unused during compilation: '-fstack-clash-protection' [-Werror,-Wunused-command-line-argument]

This makes detection of some features (-fstack-clash-protection on macOS) fail even if they are available.
This is even worst in 4.x (in the yet-to-be-merged batch), as it fails to detect "gcc pointer sign", which causes many testsuite failures.

This PR simply adds -Wno-unused-command-line-argument to the CFLAGS under gcc and Clang (and removes if from the CI workflow). Not sure if this should come earlier in the configure file ?

Should be merged in 4.x once upstream in 3.x.

@GitMensch
Copy link
Collaborator

Hm, this option was added in clang 3.1, but as expected earlier versions didn't raise a warning if its Wno form is used.
It still should not be applied to GCC, but only to clang. So maybe set it directly after recognizing clang?

@ddeclerck
Copy link
Contributor Author

ddeclerck commented Feb 10, 2025 via email

@GitMensch
Copy link
Collaborator

For some reason, I was under the impression this option also existed under GCC, but now I can't find any info supporting this.

I've just asked gcc by adding -Wunused-command-line-argument and both new and gcc told me "nope" :-)

@ddeclerck ddeclerck force-pushed the fix_unused_command_args branch from e5568c4 to 6d65085 Compare February 10, 2025 22:37
@ddeclerck
Copy link
Contributor Author

ddeclerck commented Feb 10, 2025

Updated to apply the change only to Clang.

Now for some reason (unrelated to this PR), the MSYS2 CI stopped working, complaining it can not find tex... That might be because the Windows runner image has been updated since last CI run... I guess I'll have to tackle this in another PR...

@GitMensch
Copy link
Collaborator

Can't you pin (back) the windows runner image so updating it is a manual procedure (instead of the need to potentially fix it whenever it is updated)?

configure.ac Outdated Show resolved Hide resolved
@ddeclerck ddeclerck force-pushed the fix_unused_command_args branch from 6d65085 to bdecde0 Compare February 11, 2025 08:04
@ddeclerck
Copy link
Contributor Author

Can't you pin (back) the windows runner image so updating it is a manual procedure (instead of the need to potentially fix it whenever it is updated)?

Seems GitHub Actions does not allow that (cf actions/runner#3447)...

Copy link
Collaborator

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is nearly ready for SVN - when you check that into upstream, please add a note to the Changelog as well providing the reasoning to add this option (no need to update this PR; also the CI definition here can be kept for now until that file is adjusted later - we can directly do that in the 4.x CI one, which will be rebased after this change and the init_fileio fix are merged upstream)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants